Skip to content

[SYCL][L0] Fix absence of zeInit in Level Zero LIT e2e tests #18956

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 4 commits into
base: sycl
Choose a base branch
from

Conversation

idubinov
Copy link
Contributor

No description provided.

Copy link
Member

@igchor igchor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, just replace return 1 in interop-thread with assert or something like that

@idubinov idubinov requested a review from a team as a code owner June 17, 2025 13:23
@idubinov idubinov requested a review from slawekptak June 17, 2025 13:23
@dm-vodopyanov dm-vodopyanov changed the title Fix absence of zeInit in level zero lit e2e tests [SYCL][L0] Fix absence of zeInit in Level Zero LIT e2e tests Jun 17, 2025
Comment on lines +64 to +65
// Initialize Level Zero driver is required if this test is linked
// statically with Level Zero loader, the driver will not be init otherwise.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

small nit here and below in other tests:

Suggested change
// Initialize Level Zero driver is required if this test is linked
// statically with Level Zero loader, the driver will not be init otherwise.
// Initializing Level Zero driver is required if this test is linked
// statically with Level Zero loader, otherwise the driver will not be initialized.

// statically with Level Zero loader, the driver will not be init otherwise.
ze_result_t result = zeInit(ZE_INIT_FLAG_GPU_ONLY);
if (result != ZE_RESULT_SUCCESS) {
std::cout << "zeInit failed\n";
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test use asserts, so to align with it:

Suggested change
std::cout << "zeInit failed\n";
assert((result == ZE_RESULT_SUCCESS) && "zeInit failed");

// statically with Level Zero loader, the driver will not be init otherwise.
ze_result_t result = zeInit(ZE_INIT_FLAG_GPU_ONLY);
if (result != ZE_RESULT_SUCCESS) {
std::cout << "zeInit failed\n";
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit here and in other tests below:

Suggested change
std::cout << "zeInit failed\n";
std::cerr << "zeInit failed" << std::endl;

Comment on lines +26 to +27
if (result != ZE_RESULT_SUCCESS) {
std::cout << "zeInit failed\n";
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Better to use assert here as it is already used in this test

ze_result_t result = zeInit(ZE_INIT_FLAG_GPU_ONLY);
if (result != ZE_RESULT_SUCCESS) {
std::cout << "zeInit failed\n";
return 1;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+ some small enhancement here and below:

Suggested change
return 1;
return result;

Copy link
Contributor Author

@idubinov idubinov Jun 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ZE_RESULT_SUCCESS is not equal to test's success return code. May be in current implementation both of them are equal 0, but not in general.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe print this result value to output for the future easier debugging session, or return it in other way?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants